Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add Serialize/Deserialize for ActiveValue #1631

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

videobitva
Copy link

@videobitva videobitva commented May 6, 2023

Add Serialize/Deserialize for ActiveValue

PR Info

As of right now there exists "with-json" feature flag, which allows applying changes to an ActiveModel through the JSON Value type. This could be useful when updating some data in the database by sending a JSON to the server through an HTTP API. An alternative way of doing the same thing would be to send the ActiveModel, with all its changes relative tp the Model, directly to the server. This requires the serialization of the ActiveModel. At the moment (sea-orm version 0.11) it is not possible due to Serialize/Deserialize traits not being implemented for ActiveValue. This PR adds the derive for these traits conditionally when the "with-json" feature is enabled. This way the Serialize/Deserialize traits implementation for the exact ActiveModel could be done by the library user, while the implementation for the ActiveValue is behind the feature flag.

New Features

  • Serialize/Deserialize for ActiveValue

Breaking Changes

  • No breaking changes

Changes

  • Add "serde/derive" dependency when building with "with-json" feature flag
  • Add Serialize/Deserialize for ActiveValue behind the "with-json" feature flag
  • Add proc_macro for generating Serialize/Deserialize for ActiveStruct
  • Add documentation

Add Serialize/Deserialize for ActiveValue behind the "with-json" feature flag
@videobitva videobitva changed the title Add Serialize/Deserialize for ActiveValue WIP: Add Serialize/Deserialize for ActiveValue May 6, 2023
@sphw
Copy link

sphw commented Nov 27, 2023

What's the current status of this PR? It looks like exactly what I need. I'm happy to push it over the finish line as well

@@ -36,6 +38,7 @@ pub use ActiveValue::NotSet;
/// );
/// ```
#[derive(Clone, Debug)]
#[cfg_attr(feature = "with-json", derive(Serialize, Deserialize))]
Copy link
Member

@tyt2y3 tyt2y3 Nov 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use sea_query's value_to_json to handle the Serialize part, but in general ActiveValue does not seem to lend itself for Deserialize, because the trait bound only requires T -> Value.

@tyt2y3
Copy link
Member

tyt2y3 commented Nov 28, 2023

Thank you for the contribution!

Do you really needed Deserialize? I think it's better to leave it out, because it is a complex problem (safety, customization, schema validation etc).

May be also add some test cases under /tests to demonstrate the end to end behaviour?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants